Skip to content

Conversation

@spbsoluble
Copy link
Contributor

@spbsoluble spbsoluble commented Oct 9, 2025

1.2.2

Bug Fixes

  • fix(storetypes): K8SJKS and K8SPKCS12 storetypes using a separate k8s secret for store password does not crash
    on missing or invalid secret field name.
  • fix(storetypes): K8SJKS where JKS files created using Keytool v20+ will be recognized as JKS files.
  • fix(storetypes): K8SJKS and K8SPKCS12 store/buddy passwords ending with a \n character will be trimmed to not include the newline.

Chores:

  • chore(docs): Update documentation format
  • chore(deps): Bump BouncyCastle.Cryptography to v2.6.2.

@spbsoluble spbsoluble requested a review from Copilot October 9, 2025 18:12
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@spbsoluble spbsoluble requested a review from Copilot October 17, 2025 16:25
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Copilot reviewed 26 out of 50 changed files in this pull request and generated 15 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +1331 to +1335
Logger.LogTrace("K8S Store Password show new lines: {StorePassword}", storePassword.Replace("\n","\\n"));
// remove any trailing new line characters from the string
storePassword = storePassword.TrimEnd('\r','\n');
Logger.LogTrace("Store password bytes converted to string: {StorePassword}",
storePassword); //TODO: INSECURE COMMENT OUT
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Secrets are being written to logs (both raw and with escaped newlines). Avoid logging passwords entirely. Remove these LogTrace calls or mask with fixed placeholder text.

Suggested change
Logger.LogTrace("K8S Store Password show new lines: {StorePassword}", storePassword.Replace("\n","\\n"));
// remove any trailing new line characters from the string
storePassword = storePassword.TrimEnd('\r','\n');
Logger.LogTrace("Store password bytes converted to string: {StorePassword}",
storePassword); //TODO: INSECURE COMMENT OUT
// Logger.LogTrace("K8S Store Password show new lines: {StorePassword}", storePassword.Replace("\n","\\n")); // Removed insecure logging
// remove any trailing new line characters from the string
storePassword = storePassword.TrimEnd('\r','\n');
// Logger.LogTrace("Store password bytes converted to string: {StorePassword}", storePassword); // Removed insecure logging

Copilot uses AI. Check for mistakes.
Comment on lines 640 to +642
var storePasswdString = Encoding.UTF8.GetString(storePasswordBytes);
_logger.LogTrace("Importing existing PKCS12 data with store password: {StorePassword}",
storePasswdString); //TODO: INSECURE COMMENT OUT
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logs the PKCS12 store password in clear text. Do not log secrets; remove the LogTrace or replace the value with a redacted placeholder.

Copilot uses AI. Check for mistakes.
Comment on lines +174 to +176
using var outStream = new MemoryStream();
_logger.LogDebug("Saving JKS store to MemoryStream w/ password '{Pass}'",
storePassword ?? "null"); //TODO: INSECURE - Remove this line, it is for debugging purposes only
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JKS store password is being logged. Remove secret logging; if needed, log only that a password was provided (e.g., ''). Similar secret-logging exists elsewhere in this file (e.g., lines 73-75, 81-82, 164-167, 332-335).

Copilot uses AI. Check for mistakes.

_logger.LogDebug("Adding store to list of serialized stores");
var filePath = Path.Combine(storePath, storeFileName);
_logger.LogDebug("Filepath '{Path}", filePath);
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The structured logging template is malformed; the closing quote/brace is missing. It should be: _logger.LogDebug("Filepath '{Path}'", filePath);

Suggested change
_logger.LogDebug("Filepath '{Path}", filePath);
_logger.LogDebug("Filepath '{Path}'", filePath);

Copilot uses AI. Check for mistakes.
_logger.MethodEntry(LogLevel.Debug);

_logger.LogDebug("Creating or updating PKCS12 store");
_logger.LogDebug("Creating or updating PKCS12 store for alias'{Alias}'", alias);
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor formatting issue in the log message: add a space before the placeholder. For example: _logger.LogDebug("Creating or updating PKCS12 store for alias '{Alias}'", alias);

Suggested change
_logger.LogDebug("Creating or updating PKCS12 store for alias'{Alias}'", alias);
_logger.LogDebug("Creating or updating PKCS12 store for alias '{Alias}'", alias);

Copilot uses AI. Check for mistakes.

## Discovery Job Configuration

For discovery of K8SNS stores toy can use the following params to filter the certificates that will be discovered:
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct 'toy' to 'you'.

Suggested change
For discovery of K8SNS stores toy can use the following params to filter the certificates that will be discovered:
For discovery of K8SNS stores you can use the following params to filter the certificates that will be discovered:

Copilot uses AI. Check for mistakes.

## Discovery Job Configuration

For discovery of K8SNS stores toy can use the following params to filter the certificates that will be discovered:
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct 'toy' to 'you'.

Suggested change
For discovery of K8SNS stores toy can use the following params to filter the certificates that will be discovered:
For discovery of K8SNS stores you can use the following params to filter the certificates that will be discovered:

Copilot uses AI. Check for mistakes.

## Discovery Job Configuration

For discovery of `K8SPKCS12` stores toy can use the following params to filter the certificates that will be discovered:
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct 'toy' to 'you'.

Suggested change
For discovery of `K8SPKCS12` stores toy can use the following params to filter the certificates that will be discovered:
For discovery of `K8SPKCS12` stores you can use the following params to filter the certificates that will be discovered:

Copilot uses AI. Check for mistakes.

## Discovery Job Configuration

For discovery of K8SNS stores toy can use the following params to filter the certificates that will be discovered:
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct 'toy' to 'you'.

Suggested change
For discovery of K8SNS stores toy can use the following params to filter the certificates that will be discovered:
For discovery of K8SNS stores you can use the following params to filter the certificates that will be discovered:

Copilot uses AI. Check for mistakes.

## Discovery

**NOTE:** To use discovery jobs, you must have the story type created in Keyfactor Command and the `needs_server`
Copy link

Copilot AI Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct 'story type' to 'store type'.

Suggested change
**NOTE:** To use discovery jobs, you must have the story type created in Keyfactor Command and the `needs_server`
**NOTE:** To use discovery jobs, you must have the store type created in Keyfactor Command and the `needs_server`

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants